Skip to content

feat(distributed): workers no longer need inbound ports (worker tunnel, phase 2) - #11839

Merged
mudler merged 42 commits into
test/distributed-e2e-cifrom
feat/worker-tunnel-phase1-2
Sep 2, 2026
Merged

feat(distributed): workers no longer need inbound ports (worker tunnel, phase 2)#11839
mudler merged 42 commits into
test/distributed-e2e-cifrom
feat/worker-tunnel-phase1-2

Conversation

@localai-org-maint-bot

Copy link
Copy Markdown
Collaborator

Worker tunnel: workers no longer need inbound ports (phase 2)

Distributed mode used to require every worker to expose an inbound address that the frontend dialled directly. This inverts that: a worker dials the frontend load balancer over HTTP and holds one multiplexed yamux tunnel. That tunnel lands on exactly one frontend replica, and every other replica reaches the worker by relaying through the owner. Workers now bind loopback only and advertise nothing.

This is phase 2 of the programme to remove the NATS dependency. NATS is untouched here; phases 3 to 6 move the control plane, backend installs, fan-out and the claim queue.

Why

Operators had to give each worker a routable address and open ports back to it, which is the hardest part of running LocalAI across a NAT, a VPC boundary or a laptop. After this, a worker needs one outbound HTTPS connection to the same load balancer a browser would use.

How it works

  • The worker dials GET /api/cluster/connect and holds a yamux session. gRPC, HTTP and websocket traffic are multiplexed over it.
  • Ownership of a tunnel is fenced by a PostgreSQL sequence-backed epoch. Ownership resolution joins live instances, so a dead replica can never be named as an owner.
  • A replica that does not hold a worker relays to the one that does, over the existing replica peer link.
  • Workers authenticate with a per-node credential minted at registration and stored only as a hash, replacing the previous reliance on the shared registration token.
  • If the owning replica dies, the worker reconnects and the tunnel is re-claimed by whoever it lands on.
  • A worker refuses a stream it will not serve with one of four codes, and the split is load bearing: three of them are statements about a backend that the frontend acts on (up to deleting the model's row), and the fourth says the worker learned nothing, so it reaches every reap guard as "no route". A late request frame, a tunnel being torn down and a local resource failure all use the fourth. A code an older frontend does not recognise degrades to the same safe answer.

Operator impact

  • Upgrade order matters: upgrade every frontend replica first, then restart workers one at a time. During the window, not-yet-restarted workers show healthy and heartbeating while their models return "no route". It clears on restart.
  • The reverse order fails: an old frontend rejects a new worker's registration with a 400 and the worker exits, draining the fleet node by node.
  • Rolling a frontend back requires restarting every worker, because registration is the only writer of the address columns and re-registration clears them.
  • LOCALAI_ADVERTISE_ADDR and LOCALAI_ADVERTISE_HTTP_ADDR are no longer used.
  • LOCALAI_WORKER_TUNNEL=false is now a fatal startup error rather than a degraded mode, because there is no direct-dial path left, and a worker started that way would register healthy and be permanently unreachable.
  • The container healthcheck no longer probes a port that no longer exists (this would otherwise have regressed Docker HEALTHCHECK probes a frontend endpoint, so every worker container is permanently unhealthy #10987).

The invariant this change rests on

Four failures must never be reported as each other: a routing fact, an absent connection, an unreachable peer, and an infrastructure error. Absence makes the scheduler act, reaping rows and evicting models, and one of those paths runs during inference. Removing the direct-dial fallback means a collapse between them stops being degraded and becomes unrecoverable.

Review found and fixed eight instances of that collapse: at the cluster/nodes package boundary, at five separate reaping sites, in three client decorators, in the model loader and the inference-path evicting client, in the peer link (where an expired caller deadline surfaced as "peer unreachable" under contention, reproducible in three of seven race runs), and finally one introduced by the fix for the fifth. What Dial excludes from the "no route" umbrella and what consumers exempt from "unroutable" are now the same exported predicate, so the two lists cannot drift.

Testing

  • New end-to-end suite proving inference over the tunnel, over the relay to a non-owning replica, and re-homing after the owning replica is killed, with a negative control that makes the other three meaningful: the tunnel is blocked at the balancer and the no-inbound-ports worker must be unreachable, then the block is lifted and the identical request succeeds.
  • Head-of-line blocking measured rather than assumed: 128 MiB across the session while a warm model is probed. The worst probe is between a sixth and a nineteenth of the transfer window, so the session interleaves. This is loopback, so it says nothing about a link with a real bandwidth-delay product, and the yamux windows are deliberately left at their defaults pending that data.
  • The test harness now starts one PostgreSQL container per test process (so one per ginkgo -p worker, with nothing coordinated across them) and hands out a fresh database per SetupTestDB call, dropped when the spec that asked for it ends. Isolation is unchanged; what went away is a container start per spec. It cut the cluster suite from 97s to 37s, jobs from 34s to 3s and agents from 14s to 2s.

Known follow-ups, deliberately not in this PR

  • Peer-link identity. /api/cluster/peer takes a self-declared replica id, so anything holding the shared registration token can relay to every worker a replica owns and can evict its inbound link, and can aim the roughly 31 GiB per-session peer receive window at one replica's memory. This is not a regression in kind: before this change the same token reached every worker's advertised gRPC and file-transfer ports directly. Closing it properly needs a credential minted where a replica joins the instances table, which is a migration and a design.
  • allocatePort allocates from bookkeeping only and never checks a port is free, and its default base sits inside Linux's ephemeral range.
  • BackendNode.Address and HTTPAddress remain as inert columns; removing them touches roughly 90 sites.
  • Declaring LastDialError on the backend interfaces would let embedding promote it and delete the unwrapper machinery entirely.
  • A replica with no advertised address still starts, though every worker landing there is unroutable from every other replica. It now nags loudly instead.
  • Two pre-existing -race failures unrelated to this branch: core/services/galleryop/cancellable_phase_test.go:192, and pkg/model process_exit_test.go via xlog.SetLogger.
  • The two React files in this diff ARE covered by the Playwright harness in core/http/react-ui/ (e2e/nodes-roster.spec.js and e2e/nodes-detail.spec.js drive exactly these components), but every node those specs mock has an address, so none of them exercises the new node.address || node.id fallback. That harness needs a running server and was not run here, so the fallback itself was verified by reading.

Stacked on #11812 (phase 0, the distributed e2e suite in CI), which is still open. This PR targets that branch so the diff shows only phase 2; retarget to master once #11812 merges.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y2TjpXdY7SszRrM5PhSp1e

mudler added 30 commits August 31, 2026 21:56
Replicas need to find each other to relay worker traffic, and nothing in
the tree recorded a replica's address. The advertised address is discovered
by opening a UDP socket toward PostgreSQL and reading back the local
address, which yields the interface every replica demonstrably shares
without asking an operator to configure one.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…ble addresses

DiscoverAdvertisedAddr promised to return an error rather than a fallback
no peer can dial, but only rejected an unspecified address. With PostgreSQL
on the same host or pod as a replica, which is compose, single-node and any
sidecar layout, the route to it is loopback, so every replica advertised
127.0.0.1 and a peer dialling that reached itself. Loopback, link-local and
zoned source addresses are now rejected with an error naming the remedy, and
a port outside 1-65535 is rejected before it becomes an undialable address.

Liveness was also measured on each replica's own clock: Register and
Heartbeat stamped last_seen from the Go process, and Live compared those
rows against the reading replica's time.Now(). Skew therefore shrank or
stretched the window by writerBehind+readerAhead, evicting healthy peers or
keeping dead ones. Both sides now use the database clock, which is the one
clock every replica demonstrably shares.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Returns on the first direction to finish and closes both sides so the other
unblocks; a sequential copy deadlocks on any protocol where the far side
speaks first. EOF and use-of-closed are normal termination, not errors.

The fourth spec covers a peer that stops reading mid-body, the case where a
copy is parked in Write rather than in Read. The other three tear down an
idle splice and pass even against a Splice that closes only one side.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
go-yamux/v5 matches none of its errors against net.ErrClosed, so the
classifier reported an ordinary teardown as a failure: when the session has
gone away, the FIN that Splice's own Close writes returns ErrSessionShutdown,
and a stream torn down under a live copy surfaces as ErrStreamClosed or a
reset. Splice owns that Close, so it owns the errors it produces; the
sentinels are named here rather than injected by the caller, which would make
a forgotten classifier reintroduce the same bug silently.

Cover the error half of the contract, which no in-memory pipe could reach: a
scripted stream now feeds Splice a genuine transport failure and each
closed-stream ending in turn. Replacing the tail of Splice with "return nil"
passed every previous spec.

Also assert that Splice does not return until the second direction has
finished, rename a spec that promised a leak check it never made, and correct
two comments that claimed more than the code did.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Matching yamux errors with errors.Is was too broad. Session.close hands every
live stream ErrStreamReset wrapped around whatever killed the connection, so a
keepalive timeout, a broken TCP connection or a peer that simply vanished all
matched, and a relayed request that died reported a clean ending. Nothing
upstream would have retried or logged it.

Match the plain sentinels by identity, since only identity separates a stream
that was reset from the wrapped form that means the session died. Treat a
StreamError as a per-stream reset, and a GoAwayError as normal only when it
carries the no-error code, read off ErrRemoteGoAway because the constant is
unexported. ErrSessionShutdown needs no entry of its own; it is a GoAwayError
with that code.

Order matters as much as the matching: session death wraps its cause, which is
routinely io.EOF or a closed socket, so the mux checks run before the generic
endings. Reversing them alone puts a vanished peer back to nil.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Upgrades to a WebSocket, wraps it as a yamux server session and hands it to
the caller. Rejects before upgrading so an unauthenticated dial sees a 401
rather than a WebSocket error, which is what the route-coverage test asserts.

The adapter keeps the reader of a partially consumed message across Read
calls. yamux reads through a 4 KiB bufio.Reader, so a small-payload test
cannot see a dropped message tail; the framing specs drive the adapter
directly with buffers smaller than the message.

An empty configured token authorizes nobody here, unlike the worker file
transfer server's check: this route is registered in every deployment, so
failing open would publish an unauthenticated mux.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A dead yamux session does not always arrive wrapped. Session.close publishes
shutdownErr and closes shutdownCh before it force-closes the streams, so a
Write or Close landing in that window gets the raw cause back instead
(session.go:507-510, 528-533), and for a peer that vanished the raw cause is a
bare io.EOF. The generic io.EOF clause then reported the dead session as a
clean completion.

Remove the clause. A clean read-side EOF never reached it anyway: io.Copy
consumes that and reports nil, and neither *yamux.Stream nor *net.TCPConn
takes a WriteTo/ReadFrom path that would hand one back. Every existing spec
still passes, the io.EOF entry in the normal-termination table included, which
is what showed the branch was dead for legitimate endings and live only for
the bug.

Add a spec driving a real yamux session end to end. Every mux shape until now
was a synthesized error, which is exactly why a race inside the real library
stayed invisible.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…s claimed

The prefix constant moves to core/http/auth beside the check that uses it, and
the endpoints package derives its route from there. Seven sibling endpoint
packages already import auth, so the previous direction would have deadlocked
the build as soon as this one registered in RouteFeatureRegistry, and it was
dragging echo, gorilla/websocket and yamux into unrelated service packages.

Four properties were argued in comments and held by nothing. Flipping the
empty-token check to fail open, making SetWriteDeadline a no-op, returning a
zero-length read for a zero-length message, and dropping the recover around the
callback all left the suite green. Each now fails a spec that asserts the
behaviour rather than the setter's return value.

SetWriteDeadline takes the write mutex because gorilla keeps that deadline in a
plain struct field applied at the next flush; SetReadDeadline must not take the
read mutex, since it goes straight to the net.Conn and would otherwise block
behind the read it exists to unblock.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Distinguishes a peer missing from the registry from a peer that will not
answer: the second must never be readable as node absence, or a network
hiccup between replicas evicts healthy workers.

The distinction is a property of the error type rather than of the call
sites. The unreachable error formats its cause into its message and keeps
it out of its unwrap chain, so an ErrInstanceNotFound picked up on the
dial path cannot reach a caller's absence check.

One yamux session is cached per peer and re-dialled when OpenStream on it
fails, which is how both a dead transport and a graceful remote go-away
arrive. A reset of one stream never reaches the pool, so an abandoned
request cannot cost every other worker its link.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…er's deadline

The peer link's WebSocket adapter and route constant lived in
core/http/endpoints/cluster, so the dialler in core/services/cluster had to
import an HTTP endpoints package to reach them. That pulled echo, core/http/auth
and core/config into a package whose doc says it is deliberately free of such
dependencies, and it made core/services/nodes reach an endpoints package
transitively. It also has no way forward: the worker-connect handler needs the
tunnel registry and the node token store, both of which are cycles from there.

Move WebsocketConn and PeerPath into core/services/cluster and let the endpoints
package import it, which is the direction the rest of core/http flows. The route
and the auth exemption still cannot drift apart, now asserted where both are
visible rather than by a const reference across the boundary, and the assertion
is stronger than the one it replaces: it pins the route under the prefix instead
of pinning the prefix's spelling.

Also guard the fresh-dial path with ctx.Err(), mirroring the cached path. A
caller with a 300ms deadline dialling a live, listening peer was told the peer
was unreachable, which would be enough for one impatient client to get a healthy
replica routed around once the relay consults these errors.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A worker whose link is silently broken reconnects to another replica while
the old owner's socket has not yet noticed. Without a fence both believe
they own it. Claim is a single atomic upsert returning the new epoch, and a
release must match both owner and epoch so a stale owner cannot delete a
live claim.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…reused

Release deletes the row, so a per-row `epoch + 1` restarted the numbering at 1
for the next claim. A replica could then be handed an epoch it already held:
claim w1 at epoch 1, lose the link silently, watch another replica claim and
release, reclaim and be handed 1 again, and its delayed cleanup for the first
dead link would match the live claim and delete it. The fence has to be
unique per node over time, not per row lifetime.

Every claim now draws nextval from a dedicated sequence on both the insert and
the conflict paths, so an epoch is never issued twice. The draw still happens
after the row lock on the conflict path, so the winning claim still holds the
highest epoch handed out.

Also drop last_seen. Nothing maintained it and it was always equal to
connected_at, but an indexed column named that way invites a second liveness
clock; whether the owner is alive is Instance.LastSeen, and whether a claim is
current is the epoch.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The NodeConnection model carried `default:now()`, which is PostgreSQL syntax
reaching the DDL, so AutoMigrate failed on the single-binary SQLite path and
took every SQLite caller of nodes.NewNodeRegistry down with it. Stamp the
database clock as an expression inside Claim instead, the way Register
already does, and leave the column plain.

CREATE SEQUENCE is Postgres-only for the same reason, so it is skipped on
another dialect, and Claim refuses that dialect outright: a fence that cannot
draw a token must say so rather than fail later as a missing function.

Also correct a claim the previous commit made in both the doc comment and its
message. An epoch is unique and never reissued, but it is not ordered: the
insert path draws its sequence value before taking the row lock, so a claim
that inserts after a release can be handed a lower number than one already
issued. Uniqueness is what Release needs, since it matches by equality;
callers must never compare epochs for order.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…plicas

Tasks 1 to 5 built an instances table, a splice, both halves of a peer link
and an epoch fence, and nothing in the tree called any of it: no replica
registered, no route was mounted, no sweeper ran. Proving phase 1 end to
end therefore had to start by wiring it.

A frontend in distributed mode now publishes the address its peers dial,
heartbeats it, and sweeps replicas that stopped answering along with the
connection rows they owned, in one pass so the two can never disagree about
who is alive. It serves the peer link and owns the sessions peers dial in,
refusing streams on them until phase 2 installs a relay: a session nobody
accepts on does not fail a peer's Open, it hangs it.

The address is the one peers use, not the one the process binds, and it is
derived from the route to PostgreSQL. That derivation only holds while the
database is remote, so LOCALAI_DISTRIBUTED_ADVERTISE_ADDR sets it
explicitly and a replica that can determine neither warns and keeps
serving rather than failing to start.

Three e2e scenarios run against real local-ai processes, real PostgreSQL
and real dials: replicas publish addresses that can actually be connected
to; a sibling opens a stream over the peer link and is refused without the
cluster token; and a killed replica is reported unreachable, never absent,
loses the claim it held, and takes no worker with it. Each was verified by
mutation: eight injected defects, each failing the scenario that claims to
catch it.

Also moves RegisterClusterRoutes to core/http/routes beside every other
registrar, folds AutoMigrate and the epoch sequence into one
cluster.Migrate, and turns the peer route's auth-coverage spec into a real
assertion: it drives the request through the actual auth middleware
instead of comparing two string constants, which the old spec would have
passed even with the exemption deleted.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review found the recurring class: assertions that a wrong implementation
also satisfies.

The "refuse promptly, never park the peer" guarantee was stated in three
places and tested in none. Removing the Close from the no-relay branch left
the whole cluster suite green, because the specs asserted only that some
error arrived and yamux reports a read deadline as ErrTimeout: a parked
stream satisfied that as well as a refused one. Both specs now require an
ENDING, EOF or a reset, inside a deadline short enough that parking is
unmistakable, and both go red when the Close is removed.

Deregistration existed only in a comment. Membership.Stop ended the loop and
left the row behind, so every clean rolling restart had peers dialling a
corpse for the full liveness window; the shutdown comment described the
opposite. Registry.Deregister deletes the row and the connections that
replica owned, in one transaction, for the reason the sweeper does both, and
an e2e spec pins departure inside a budget shorter than the liveness window
so it cannot pass on the sweeper doing the work. Before: the spec times out
with both replicas still live. After: 3.6s.

The configured advertised address bypassed every check discovery makes, so
the one value most likely to be copied between hosts, 127.0.0.1, was taken
verbatim and would make every peer dial itself. Both paths now share one
rejection rule: unparseable is refused, "this host" is warned about once and
honoured, because a single-host deployment uses it correctly.

Two comments claimed more than the code does. The sweeper said a stalled
replica recovers via re-register; only its instance row does, while the
connections another replica reaped stay gone and the sockets stay held here
- phase 2 must re-claim, on re-register, every connection a replica still
holds locally. And Owner became OwnerRow, documenting that the owner it
names may be dead for up to InstanceLiveness plus a heartbeat and that any
caller acting on it must join instances itself, so the deferred constraint
lives at the call site rather than in a report; the plain name is left free
for the joining version.

Minors: warn once when the peer link mounts with no registration token, so
an operator sees the cause rather than 401s; Stop no longer blocks forever
when Start was never called; corrected the NewRegistry migration doc and an
e2e comment that described a 6s window as "throughout".

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
ReapStale deleted from instances then node_connections while Deregister took
them the other way round, both inside one transaction and both running
concurrently by design: a replica shuts down while a peer sweeps it. Opposite
orders let each hold the row the other waits for. PostgreSQL breaks the cycle by
aborting one side, so the cost today is a warning rather than lost data, but the
inversion costs nothing to remove.

Deregister now deletes the instance row first. That is the order ReapStale is
forced into anyway, since its connection delete asks which instance rows
survived, so the sweeper is the fixed side. Both functions say the order is
deliberate and shared, and name the other. A spec records the statements each
path issues and asserts they delete from the same two tables in the same order;
racing two transactions until they really deadlock would be flaky and could pass
for the wrong reason.

The rest is comment and spec accuracy, deferred from the phase 1 task reviews:

- co-location does not imply loopback. Compose's usual host=postgres resolves to
  a bridge address and discovery works there; it is a DSN that NAMES localhost
  that yields a loopback source address. Corrected in the DiscoverAdvertisedAddr
  doc and in the spec comment that repeated it.
- unroutableReason labelled every scoped address "link-local", including the
  class the check exists for, and formatted the IP with %s, which drops the
  %iface, so the reported address was not the one being rejected. Split into two
  cases, both rendered with their zone. CheckAdvertisedAddr passed zone "" and
  net.ParseIP rejects fe80::1%eth0, so a scoped literal looked like a name and
  collected no warning at all; the zone is now split off before parsing.
- Splice's "Both callers satisfy it" claimed callers that still do not exist.
  It now names the two stream types the wake-on-Close property was verified
  against and says a phase 2 caller over anything else has to check it.
- restored, short, why a socket-level ECONNRESET stays reported while a yamux
  reset does not: the yamux endings are the teardown Splice's own Close
  provokes, and whether an aborted request is routine is the relay's policy.
- the real-yamux spec's far.Read had no deadline, so a stall parked the suite
  rather than failing it.
- gorilla's SetWriteDeadline is conn.go:796, not 787.
- ClusterPathPrefix is no longer derived from: the peer route spells its path
  out, because core/services/cluster must not import core/http/auth. The comment
  now points at the spec that holds them together instead of claiming a
  derivation the move removed.
- the epoch spec asserted e2 > e1, an ordering Claim's doc tells callers not to
  rely on. It asserts uniqueness, which is what the fence guarantees, and is
  named for that. A sibling spec still described the epoch as incrementing in
  SQL when it is drawn from a sequence.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
OwnerRow is a bare row read of node_connections. A connection row outlives the
replica that wrote it: a replica that dies stops heartbeating, but its rows
survive until a peer's sweep removes them, which is up to InstanceLiveness plus
one InstanceHeartbeat later. For that whole window the table names a process
that is gone. The next component phase 2 builds is the relaying dialer, and a
dialer reading OwnerRow would relay into a corpse for roughly 35 seconds after
every replica death, then report the worker as unreachable when it is in fact
absent, which is the distinction the phase 1 end-to-end specs pinned.

Owner is the resolving read: one statement joining instances, returning
ErrNoConnection when the row is missing OR its owner is not live. Both cases are
one answer on purpose, since both mean no replica here holds this tunnel; they
differ only in which sweep has run. It is one statement, not a row read followed
by an instance lookup, because between two statements the owner can die and the
caller would act on an owner the second read would have rejected.

OwnerRow stays, unjoined, for readers that need the row itself, and a spec holds
the two apart: with an aged-out owner, OwnerRow still names it and Owner
refuses, so neither can quietly become the other.

The liveness predicate is now one string, instanceIsLive, shared by Live and by
Owner's join. Two spellings of one fact drift, and this drift would show as a
relay to a replica one query calls dead and another calls alive. It is
table-qualified so it is unambiguous inside the join, and the cutoff stays on
the database clock, so replica clock skew cannot widen or narrow the window.

Both mutations were run. Dropping the liveness predicate from the join fails 3
specs, the aged-owner one among them. Replacing the database clock with a
Go-side time.Now() fails 1: the aged-owner specs still pass, because the two
clocks agree on one host, and only the recorded-SQL spec sees the literal
timestamp. That is why that spec exists.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review round 1 on the joined Owner read. The behaviour was accepted; three
comments claimed more than the code delivered, one spec pinned less than its doc
promised, and one pre-existing spec ranked epochs.

instanceIsLive said every reader of instance liveness uses it, which was false:
ReapStale spelled the complement by hand. The complement is now written as
NOT (instanceIsLive), so "stale" is exactly "not live", including how each side
treats a NULL last_seen, and the sentence is true. Inverting that predicate
fails 3 reaper specs, so the routing is held.

The Select("node_connections.*") in Owner was justified by a SELECT * hazard
that cannot occur: with a join present and nothing selected, gorm expands the
model's own columns table-qualified (callbacks.BuildQuerySQL), and the suite is
green with the Select removed. It stays, because the projection should be a
property of this query, and the comment now says that instead.

Owner gained the dialect guard Claim has. now() and make_interval are
PostgreSQL, so on the SQLite single-binary path it failed with "no such
function: now", which reads as a missing migration; that regression already
shipped once in phase 1. The refusal is deliberately not ErrNoConnection: a
deployment with no cluster has no answer about ownership, and reporting absence
would let a caller conclude the worker is not connected. A spec in the
non-PostgreSQL block holds all three properties.

The new specs aged rows by ten minutes, which any window between zero and ten
minutes satisfies, so nothing tied Owner's window to the one the sweeper uses.
They now age to just past InstanceLiveness, and a sibling ages to half of it and
must still resolve. Widening the window tenfold fails 2 specs, narrowing it
tenfold fails 1; before this both were silent.

The concurrent-claim spec asserted the stored epoch was the highest handed out,
and justified it with claims drawing their epoch after the row lock, which
contradicts Claim's own doc: the insert path draws nextval while the tuple is
built. It now asserts the stored epoch is one of the epochs handed out, and
ranks nothing.

OwnerRow's doc justified the function with a sweeper that does not call it.
ReapStale deletes orphans with a set difference; the callers are this package's
specs and one e2e assertion. It says that, and states plainly that a caller
needing to know who owns a node in order to dial it wants Owner.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Phase 1 left the connection fence with a table and no sockets behind it.
This adds the registry that holds them: Attach claims the node and then
stores the session, Open hands out a stream over the tunnel this replica
holds, Detach releases the claim it was handed, and Held names what this
process is carrying.

The claim is written before the session is stored. A claimant that
installs itself and only then finds it cannot claim has, for that window,
published a tunnel no row records, so Held names it while a peer asking
Owner is told the worker is connected nowhere.

ErrNotOwner is produced at one place, the map miss. It is a routing fact:
some other replica may hold that worker perfectly well. A broken socket
under a held entry is returned as itself, because answering "not held
here" would send a dialer looking elsewhere for a worker this replica is
holding.

Epochs are compared for equality and never ordered. Claim guarantees an
epoch is unique and never reissued; it does not guarantee the later claim
draws the larger number, because the sequence value on the insert path is
drawn before the row lock.

The membership loop now re-claims on re-register, which closes the hole
phase 1 named in ReapStale. A replica that stalls long enough is swept by
a peer, losing its instance row and, in the same transaction, every
connection it owned; Register rebuilds the instance row and nothing else,
so without this it serves workers that every other replica reports as
connected nowhere. Re-claiming draws a fresh epoch, so an attachment
carries two: the token Attach handed back, which is what Detach matches
and which never moves, and the epoch of the row currently held, which is
what Release is given. Collapsing them would leave the re-claimed row
outliving the socket with no caller able to remove it.

A tunnel whose session is already closed is skipped rather than claimed
back, because claiming is an upsert and would take the row from whoever
holds the worker now.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Two Attach calls for one node both claim, and PostgreSQL serialises the
two upserts, but nothing ordered the two map writes against the two
commits. The entry left installed could be the one whose claim lost the
row, and its Detach then released an epoch the row does not carry, so the
release matched nothing and the row survived the socket.

Nothing swept that row. This replica is alive and heartbeating, so
ReapStale leaves its rows alone, and no reconnect is coming for a worker
that has gone. Owner kept naming this replica as the live owner of a
tunnel it no longer held, and every dialer sent here was answered
ErrNotOwner, which is the relay into a replica that cannot serve the
request that this phase exists to prevent.

Claims for one node now pass through a gate, so claim and record are
indivisible. It is per node rather than one lock over the registry, the
way PeerPool locks per peer: the claim is a database round trip, and a
slow one for a single worker must not hold up Open for every other.
Detach is not gated, because it takes no context and must never park
behind an in-flight database call, and it changes no epoch.

Reclaim takes the same gate, which makes its claim the newest one for
that node, so it records the epoch on whatever attachment is installed
rather than only on the one it listed. Refusing to record onto an
attachment that replaced the listed one would leave that row with nothing
able to release it. The interleave the gate does not cover is Detach, and
a claim whose attachment detached while it was in flight is now released
again rather than left behind.

Also: restore Start's doc comment, which SetTunnels had swallowed; keep
reaping other replicas when this one fails to rebuild its own row, rather
than skipping the sweep along with the re-claim; scope the comment about
an unnoticed dead socket to the keepalive of the session whoever accepted
the tunnel built, since the worker session config does not exist yet; and
pin the sortedness of Held, the nil-session refusal, and both re-claim
interleaves with specs.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The gate is justified by being held for one claim round trip, and Attach
held it across the close of the session it superseded. Closing a yamux
session closes the underlying conn and then waits for both its send and
recv loops to exit, and the send loop can be inside a write bounded only
by ConnectionWriteTimeout, so that is a wait on other goroutines. It must
not stand between a worker re-dialling this node and its claim.

The gate is now released after the store and before the close, which also
makes Attach match reclaimOne, where it has always been released
explicitly on every path. This is safe because a superseded session is no
longer reachable from the map by the time it is closed: the next re-dial
replaces an entry that already names the new session.

Pin the re-claim half of the gate too. A worker that re-dials between a
re-claim's commit and its record leaves the row carrying the re-dial's
epoch while the entry carries the re-claim's, so the attachment holding
the socket releases an epoch the row does not have and the row outlives
it, with nothing to sweep it while this replica is alive. Only Attach's
half of the serialisation was asserted; keying the two apart left every
spec green.

Also take the test hook's action under the lock that guards whether it
has fired. It was written from the spec's goroutine and read from
whichever goroutine issued the statement, which is a race in the harness
that pins the serialisation specs.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A worker needs no inbound port: it dials GET /api/cluster/connect, the
connection becomes one multiplexed yamux session, and the frontend opens a
stream on it per request. This adds the endpoint that accepts that dial and
attaches it to the tunnel registry.

The dial is authenticated against the NODE's own stored token hash rather than
the deployment's registration token. That is the mechanism, not yet the
isolation, since a worker still registers by presenting the shared token; what
it rules out is the shortcut of comparing against the configured value, which
would have to be unpicked the day workers get their own secrets.

Every refusal happens BEFORE the WebSocket upgrade, so a dialer reads an HTTP
status rather than a handshake error. The route is registered in every
deployment, single-binary ones included, which is what puts it in front of the
route-coverage test that holds that rule in place; with no node registry it
refuses every dial, and tells a credentialed one the frontend has no cluster
rather than that its token is wrong.

A lookup that FAILED is answered as a failure. Reporting a database that could
not be read as "unauthorized" would send a worker re-registering, throwing away
the identity its tunnel and loaded models are keyed by.

Wires the tunnel registry in core/application/distributed.go and hands it to
the membership loop. Without that call the re-claim after a replica is reaped
had no production caller and could never run.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
…per spec

Review follow-up. Twelve findings, none blocking, grouped here by what they
protect.

Panics. The handler now recovers between the WebSocket upgrade and the
hand-off, the way the peer link next door already did: net/http recovers the
panic but leaves the hijacked socket open, so without this a worker keeps a
session this replica has no entry for and will never detach. The claim gate in
Attach and reclaimOne is now released with defer, so a panic under Claim cannot
wedge one node's gate for the life of the process. SetTunnels gained the
nil-receiver guard its sibling Stop has.

Operability. A deployment with no registration token stores an empty token_hash
on every worker, so every tunnel dial 401s forever on a frontend that looks
correctly configured. That now warns at startup, logs its own line rather than
sharing the "wrong token" one, and is stated in the docs together with the fact
that setting the token later needs the workers to register again.

Authorization. A node still awaiting admin approval is refused with 403. The
rest of /api/node/ gates on nothing, but the two places that hand a node
something durable, its API key and its NATS credential, both refuse a pending
one, and a tunnel is that kind of grant. Draining and unhealthy nodes keep
their tunnels on purpose.

Comments that claimed more than the code. The global auth middleware does run
on this path and then declines to reject; the future per-node secret only lands
without a change here if it lands in TokenHash; the empty-hash guard is
defensive rather than deciding; ClusterPathPrefix is no longer only
replica-to-replica; the docs no longer say a reaped replica re-claims
unconditionally.

And the test harness. SetupTestDB started a PostgreSQL container per BeforeEach
with a readiness deadline it asserted on, which is one chance per spec to fail
one spec inside its setup, anywhere, never twice in the same place: the shape of
the flake seen twice here and never reproduced. It now starts one container per
process and creates a database per call, which is the pattern tests/e2e already
proved. Isolation is unchanged and is now asserted for the first time. All 69
call sites are untouched; the eleven consumer packages run 1404 specs green, and
jobs went from 34.3s to 3.3s, agents from 13.8s to 1.9s, cluster from 97.4s to
37.5s.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
…ressions

Two advisory-lock specs named their database by literal, ALTER DATABASE testdb.
Once the test helper started handing every spec its own database on a shared
server, that statement landed on the maintenance database and did nothing to the
one the spec was holding, so both specs went green having never reproduced the
condition they exist for. They regress a model-load advisory-lock wedge that has
already shipped to production once, so the previous commit's de-flaking silently
disarmed a regression test for a real deployed bug.

Both sites now read the name back with current_database() and, more importantly,
assert the override actually landed before relying on it. A literal name can go
stale again; an assertion that the setting is in force cannot pass while it is
not. Removing either production override now fails the matching spec with the
real 55P03 and 57014 again.

That literal also meant every CREATE DATABASE and every DROP ... WITH (FORCE)
ran under the 300ms bound it set on the maintenance database, which is a new
load-dependent single-spec flake inside the change that was meant to remove one.
The helper's maintenance connections now pin one connection and clear both
timeouts on it, so no setting a spec makes can bound them, and a white-box spec
imposes the leak deliberately and proves it does not reach them.

Also pins the reclaimOne gate deferral the previous commit added without a test,
by panicking inside the re-claim's own claim statement, and drops the per-dial
empty-token log line to debug now that the boot warning says it once.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The guard added last commit was circular. It cleared the maintenance database's
timeouts by executing SET statement_timeout = 0 on a connection that had already
inherited that database's bound, so the statement clearing the bound ran under
the bound it was clearing. Under the white-box spec's deliberate 1ms that gave
it 1ms, and it failed roughly once in fifty at 8-way concurrency with SQLSTATE
57014. The guard against invisible load-dependent flakes had become one.

The clearing is now delivered as a connection startup option, options=-c
statement_timeout=0 -c lock_timeout=0 on the maintenance DSN, so there is no
statement left to abort. Raising the imposed bound would only have bought
headroom and left the circularity in place. pgx puts every URL query parameter
into settings, options is absent from notRuntimeParams so it becomes a runtime
parameter, and runtime parameters are copied into the startup message
(pgconn/config.go:340-378, 606-617; pgconn/pgconn.go:382-388).

The spec now discriminates on pg_settings.reset_val, the value in force when the
connection started: 0 for a startup option, 1ms for a session SET. A first
attempt using a deliberately slow first statement did NOT discriminate, because
under the circular design the SET is itself the first statement, so by the time
a spec runs anything the session is already unbounded. Reinstating the circular
clearing now reddens the spec deterministically rather than intermittently.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
… own

The worker end of the tunnel. It dials wss://<register-to>/api/cluster/connect,
holds one yamux session as the CLIENT, and serves every stream the frontend
opens on it. Nothing dials into the worker, which is the point: no inbound port,
no reachable address.

Each stream opens with a length-prefixed frame naming a tag and a target, and
the worker answers before either side speaks the tunnelled protocol. The reply
is sent on every stream, not only on refusal, because the protocols carried here
are client-speaks-first and a reply sent only sometimes would arrive interleaved
with a response body. Two tags today: grpc reaches a backend process, and only
on 127.0.0.1 within this worker's own backend port range, because a tunnel
terminates inside the worker and letting the frontend name a host would make
every worker a proxy into its own LAN; http reaches the worker's file-transfer
server, whose address the frontend is not asked about.

An unknown tag, an unreachable local service and an unparseable request are
three refusals and stay three on the wire. A frontend gives up on the first and
retries the second. Each is answered AND the stream is ended: a worker that says
why and leaves the stream open has parked the caller on a request nobody will
answer, and a deadline on the far side cannot tell that from a slow worker. The
specs assert the stream ends rather than that an error occurred, which is what
phase 1 shipped in three places and held in none.

Reconnects double from 500ms to a 30s ceiling, each wait drawn between half the
interval and all of it, and the interval returns to its floor only after a
session that LASTED. Resetting on connect is how a rolling restart, where every
dial succeeds and dies moments later, becomes a retry storm against the first
replica back up. Nothing is assumed to survive a reconnect: the credential is
read at dial time, never captured.

And the credential is now real. The tunnel endpoint advertised authenticating a
worker against its own secret, but registration stored the hash of the shared
registration token, so a leak plus a known node ID still opened a tunnel.
Registration now mints a per-node secret, returns the plaintext once as
tunnel_token, and stores only its SHA-256 in a new column; the endpoint compares
against that and does not fall back to the old one. Rotating on every
registration follows from storing only the hash, since a re-registering worker
cannot be told the secret it already holds; its live tunnel is unaffected,
because the credential is checked when a tunnel is dialled and never again.

Unlike the agent API key and the NATS JWT next to it, the credential IS issued
to a node awaiting approval: the tunnel route re-reads the node's status on
every dial and refuses a pending one, so it is inert until an admin acts, and
withholding it would strand every worker that registers exactly once.

A node that has not registered since this change cannot tunnel, and the column
cannot be back-filled because the plaintext only ever existed in the response
that minted it. The boot warning that said tunnels need LOCALAI_REGISTRATION_TOKEN
is replaced: it was true while the tunnel authenticated against that token's
hash, and says the wrong thing now. What is still true, and is what it warns
about instead, is that without one, registration itself is unauthenticated.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…dary

Review follow-up. One blocking finding and seven others.

The blocking one first, and it is this project's recurring shape: the untested
path. loopbackService is the function whose comment calls the discarded host
"the security property this function exists for", and nothing tested it. The
reviewer replaced its body with a dial of whatever the frontend named, no port
range, and all 131 specs passed. Every spec installed the permissive test
dialler, so the real routing table was exercised nowhere.

It now has specs, and the property is stated as reachability rather than as a
property of the code: a listener on 127.0.0.2 that only the frontend's target
names must NOT be reached. Plus the port-range table, fixedService, loopbackAddr,
tunnelEndpoint, and the table itself, which moved out of Run into tunnelServices
so it can be built without starting a worker. One spec drives a real stream
through that table over the wire, so the routing rules are exercised end to end
at least once rather than only in isolation. The reviewer's mutation now reddens
ten specs, and six narrower ones redden between two and four each, so no spec is
riding on another.

The shape changed too, not only the coverage. The dial address is built from a
loopbackHost constant and strconv.Itoa of a validated int, so nothing derived
from the wire reaches DialContext at all: restoring the hole takes ADDING a data
flow, not deleting a check.

And a taxonomy fix found while specifying it. A port outside this worker's
allocator range was reported as unavailable, which tells a frontend to retry
something that can never work. It is a bad request now, and a backend that is
merely not listening yet stays unavailable, which is the retryable one.

Agent nodes no longer get a tunnel credential. Nothing dials into an agent
worker, so a tunnel replaces nothing for it and no client would open one, and
the gate is at the mint site rather than in the handler: with no credential
minted the hash stays empty and the existing empty-hash refusal covers it, so
enforcement is structural.

Two comments and one doc paragraph said an anonymous registrant gets a "working"
credential. With auto-approve off the node is pending and the credential is
inert, which is the distinction this same change argues three files away to
justify minting for pending nodes at all.

A refusal reason over the frame limit was cut on a byte boundary and could split
a rune. It cuts on a rune boundary now, and the code survives truncation, which
is what keeps a refusal classifiable.

Also: the pending-node spec asserted only that a credential was non-empty, so a
credential derived from the shared token passed it; it now pins per-node-ness the
way the headline spec does. The tunnel handler's citations into nodes.go were
stale before this branch landed, having been written against a file the same
commit was editing, and are by function name now. The static-NATS path says
plainly that an externally forced rotation locks it out until restart, and where
that gets fixed. tunnelproto gained direct specs, including that a read failure
is never reported as a refusal.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
…uctural

Re-review follow-up, three items. Two are the overclaiming-comment class again,
and the first is that class with a real defect underneath it.

attachTunnelToken said "enforcement is therefore structural": an ineligible node
never gets a credential, so its hash stays empty and the tunnel route's
empty-hash branch does the refusing. That was true for a node that had always
been an agent and false for one that had not. Register upserts by NAME, so a
backend node re-registering as an agent keeps its ID, and Register's struct
Updates zero-skips the credential column while writing the new node_type. The
early return left the credential the node earned as a backend sitting on a row
that is now an agent, and ConnectHandler never looks at node_type.

Fixed by making the claim true rather than by softening it, because the mint-site
gate was chosen precisely on the grounds that it was structural: an ineligible
node now has its column CLEARED, unconditionally, so the invariant does not
depend on what the row happened to contain. A spec pins it and was red before the
change. Same shape as the Register-upserts-by-name hazard already carried
forward: a name is not an identity.

Second, loopbackHost claimed to be the only host any tunnel stream is ever
dialled on. It is not: fixedService dials whatever Run built it from, which is
this worker's own LOCALAI_HTTP_ADDR, and loopbackAddr rewrites only a wildcard
bind, so an operator who binds the file-transfer server to a routable address
gets a routable dial. The property that matters is narrower and is what the
comment says now: the frontend cannot STEER the dial. The grpc tag builds its
address from a constant and a validated port with nothing from the wire reaching
the dialler, and the http tag ignores its target entirely. Worth stating exactly
rather than summarising, because the argument about what a stream can reach rests
on knowing which hosts are reachable, and an overstatement at that site is what
would let someone conclude the constant alone is doing the work.

Third, a spec named "without allocating it" measured no allocation. It now
asserts the mechanism the defence actually rests on, that the reader consumes the
two length bytes and not one byte of the body, through a counting reader. The
input carries a body on purpose: against input that ends after the header the
assertion would pass with the limit check deleted.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
A worker holds ONE tunnel and it lands on ONE frontend replica, so with N
replicas behind a load balancer roughly (N-1)/N of requests arrive somewhere
that cannot reach the worker directly. This is the piece that carries them:
the SessionStore stream handler reads which worker a peer's stream is for,
opens a stream on the tunnel this replica holds, and splices the two.

Splice has had no production caller since phase 1. It has one now, and being
the first caller it settles the two endings phase 1 deliberately left open,
both of which read as normal termination until now:

  - a peer-initiated *StreamError{Remote: true}, which yamux builds only from
    an RST frame the far side sent (stream.go:432-449); a reset this side asks
    for carries Remote: false, and Splice never resets anything, its own Close
    sending a FIN;
  - a graceful ErrRemoteGoAway, which handleGoAway returns for code
    goAwayNormal (session.go:829-833) and close hands unwrapped to every live
    stream (session.go:328-337).

Both truncate whatever was in flight. Reporting them as normal termination is
how a half-finished inference comes to look like a short one that completed,
so both are now reported; the local forms stay silent, because those are the
teardown Splice provokes itself. The decision cannot live in a caller reading
Splice's result, since a result already mapped to nil carries nothing left to
reclassify, so it lives at the classifier with the reasoning beside it. The
relay logs it at debug: a client cancelling a relayed request produces one per
cancellation, and the truncation is separately visible to the frontend's own
gRPC or HTTP client.

The relay hop gets its own request and reply frames. They have to be distinct
from the worker tunnel's, because a relayed stream carries both hops' frames
back to back, and a vocabulary shared between them would let a reader applied
to the wrong hop hand back a plausible sentinel belonging to the other. Its
three refusals stay apart for the reason the worker's three do: ErrNotOwner is
a routing fact and the caller should resolve the owner again; unavailable is
infrastructure at this replica and a retry is worth something; bad-request is
the caller's bug. None of them is, or may be built over, an absence error.

One hop, always. A stream naming a worker this replica does not hold is
refused, never resolved and relayed onward, so a stale ownership row cannot
become a loop between two replicas each certain the other holds the worker.

PeerPool is constructed and closed alongside SessionStore, so both halves of
the peer mesh now have an owner and a shutdown.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…gets

Review round 1 on task 5. Eight non-blocking items, all addressed.

The classifier read Remote in two predicates with a report-by-default
fallthrough behind them, so reverting either read left the whole suite green:
the error reached the same answer down the other path. A correctness argument
that rests on mutation evidence cannot afford a shape that cannot be mutated in
pieces, so the two predicates collapse into one muxVerdict deciding each error
type once. Falsifying either Remote read now reddens exactly one spec.

Three claims the comments made loudly and nothing tested:

  - clearing the header read deadline before the splice. Deleting the clear
    left all 49 focused specs green, while in production it is the difference
    between a relayed response that streams for an hour and one that dies after
    fifteen seconds of quiet;
  - the open budget bounding the open and nothing after it;
  - closing the worker-side stream when the acceptance reply cannot be
    written, which leaks one stream on the worker per failure.

All three are pinned now. The first two share a spec that sets both budgets to
50ms and then watches the conversation outlive them by ten times, which is an
assertion about an event that must not happen and so is the one wait a channel
cannot replace. The third drives the relay with a peer stream that delivers a
request and then fails every write, because no pair of live yamux sessions can
be made to fail that write on cue.

The disjoint-vocabulary argument was specced for the accepted frame only. Both
refusal directions are covered now, and asserted as "not one of the other hop's
sentinels" rather than merely "an error", since reading a relay refusal with the
tunnel's reader always errors and the question is whether it errors as the wrong
thing.

The open budget stays non-configurable, and says so: the number that matters is
how long the original client will wait, which is not known on this side and is
not something a deployment-wide constant can stand in for. The honest fix is the
caller's remaining budget travelling in the request frame, which belongs to the
dialler that has the budget.

Two comments corrected: nothing deadlines the peer stream after the clear, so
the tunnelled protocol's own deadlines cannot be what justifies clearing it; and
the membership sweep deletes departed replicas but reports only how many, so
identifying them is work that would have to be done, not knowledge waiting to be
plumbed. Recorded at muxVerdict: a remote RST that does not ride a
typeWindowUpdate frame yields the bare sentinel and is still silenced, which is
unreachable between two go-yamux peers but keeps the new rule from reading as
unconditional.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Opus 5 [claude-code]
The tunnel, the fence, the registry and the relay were all built and none of
them carried a byte: every dial from the frontend still went to the address a
worker registered. This is where that stops. One WorkerDialer resolves where a
worker's tunnel is held, opens a stream on it locally or relays through the
owning replica, and hands back a conn past both handshakes; gRPC, the file
stager's HTTP client and the log-streaming WebSocket are all pointed at it.

A worker's address stops being somewhere to connect to and becomes the name of
which backend process a stream is for. It still appears in URLs, logs and
errors, because that is what identifies the process; what it no longer decides
is where the bytes go.

Nothing falls back to dialling it. BackendClientFactory now has exactly one
method, NewClientForNode, and returns an error where there is no way to reach
the worker. The direct-dial constructor was removed rather than kept beside it,
because leaving one on the interface keeps the bypass one word away from every
call site that holds an address, which is all of them.

The second construction path is closed too. DistributedModelStore built remote
models with a nil client, and pkg/model.Model.GRPC then dialled the raw address
lazily on first use - reached in production by ShutdownModel's Free and by the
backend monitor's Status. Those models now carry the tunnel-backed client, and
a model that cannot be given one is logged and not listed.

Four conditions stay unmixable, and one path produces absence: the dialer
answers ErrNoConnection only where Owner's liveness join did. A peer that will
not answer, a stale ownership row, a worker's own refusal and a missing relay
path are each reported as themselves. This matters because nodes ACTS on
absence, and the collapse would have it reclaim the models of a worker that is
connected and busy.

That is not hypothetical. Writing the mutation for it exposed the bug in this
change's own first draft: probeHealth returned bare false when it could not
build a client, and tryWarmPath deletes the replica row on a false probe. A
frontend whose dialer broke would have emptied node_models for the whole
deployment while every model kept running. probeHealth now returns alive and
probed separately, the reconciler gets a ProbeUnknown outcome that neither
advances nor clears a failure streak, and the health monitor skips rather than
counting a miss.

Task 5 left the relay's open timeout at a fixed 15s and said so: no operator
has the information to set it, because the number that matters is the original
client's remaining budget, which is invisible on the relay side. The dialer has
that budget, so it now states it in the relay request frame and the owner takes
the smaller of the two. It can only shorten - a patient client must not be able
to park a relay goroutine and a stream slot on a worker that stopped accepting.
Zero is written as no budget at all, since on the far side the number zero is a
caller with nothing left and would refuse healthy traffic.

Seven mutations, each reddening a named spec: peer-unreachable as absence; the
local-failure guard dropped; max instead of min on the budget; the nil-client
model restored; ProbeUnknown falling through to the reaper; OwnerRow instead of
Owner; probed collapsed into alive. The first budget spec passed for the wrong
reason - a handshake deadline, not the relay - and was replaced by three that
each assert one link, including one where the spec plays the owning replica and
reads the budget out of the frame instead of inferring it from a clock.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…of the package

Review round 1 on task 6. Five blocking findings, all with the same root: the
conditions the dialer kept apart were erased one layer out, because every one of
them arrived at core/services/nodes as a gRPC codes.Unavailable, which is also
what a backend process that died produces. Four call sites acted on that by
deleting a replica row, one of them after a single failed probe.

The fifth condition is ErrNoRoute: this replica could not get a request to a
worker's backend, and no claim at all about the worker. A worker's presence is
its HEARTBEAT, which nodes owns; a route is a separate fact that cluster owns,
and the two now differ. They differ in normal operation, not exotically: a
worker that has not dialled its tunnel yet after a frontend-first upgrade is
unroutable on every request while it heartbeats and serves.

Two properties, both mutation-tested. Every failure to resolve or open a route
carries ErrNoRoute, so a consumer has one check to make. No failure carries an
absence sentinel: routeFailure is the single place that rule lives, and it keeps
ErrNoConnection and ErrInstanceNotFound in the message and out of the unwrap
chain, the guarantee unreachableError already made for peers. Everything else
stays matchable, so ErrNotOwner and ErrPeerUnreachable are unchanged for anyone
who can act on them. A worker's own refusal carries no umbrella, because a
worker that answers has demonstrated it is there and that is the only real
evidence on the path.

Crossing the boundary needed a value, not a code. NewClientWithDialer wraps the
dialer and records each outcome; LastDialError hands it back behind a narrow
interface, and nodes.unroutable turns it into ErrWorkerUnroutable with the
cluster sentinels still in the chain. A spec asserts a dial failing with
ErrNoRoute plus ErrPeerUnreachable arrives matching all three and matching
neither absence sentinel.

The sweep found a fourth site the review had not named: pkg/model checkIsLoaded
evicts a remote model on a connection error, and a tunnel dial failure is one.
Four other reap sites were cleared with reasons - inflight and the worker
authoritative pass reap only on semantic answers, scale-down is driven by
last_used, abandoned loads decide on the node's heartbeat. Every fixed site also
grew the opposite spec, so the new check cannot pass by never reaping.

probeCache carries the reason through singleflight rather than a closed-over
variable. A variable is only written by the goroutine that runs the probe, so
the leader would correctly decline to reap while every joiner reaped on the
leader's own observation; a mutation reproduces exactly that.

The docs sentence promising LOCALAI_WORKER_TUNNEL=false restores direct dialling
is gone. There is no such path, so it said the operator could take a worker dark
and call it a rollback. Replaced with the upgrade order that is actually safe.

The deadline spec the reviewer found vacuous now waits on the dial context's own
Done channel before touching the stream, so the armed deadline has really
expired; the mutation that survived for the reviewer reddens it.

Nine mutations, each reddening a named spec, including both halves of
isAbsenceClaim independently.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…st gRPC

Re-review round 2. One blocking defect, and it was the concern I filed myself
last round and mis-scoped as a future trap. It was live, and it sat on the most
destructive reaping path of the five.

RouteResult.Client is an InFlightTrackingClient, over a FileStagingClient when a
stager is configured. model_router puts that on the cached remote model and
pkg/model's checkIsLoaded asks IT whether the transport failed. Both wrappers
embed grpc.Backend, which does not declare LastDialError, so the type assertion
read nil and the guard added last round fell straight through to the old
eviction. That eviction sends backend.stop over NATS to every node holding the
model and deletes every replica row, where the other sites delete one. The spec
covering it built a bare client by hand, which is why it passed while production
did not.

This is the third time in this task a correct fix was disarmed one layer out, so
the fix is a mechanism rather than two methods. BackendUnwrapper is one line per
decorator, LastDialErrorOf walks the chain, and both consumers now call it
instead of each keeping its own assertion. One implementation, no per-caller
policy to get wrong.

Sweeping every type that embeds or holds a grpc.Backend found a third decorator
the review had not named, and it is itself a reaping consumer of the same
collapsed signal. ConnectionEvictingClient is built for remote models in
initializers.go and its evict callback runs ShutdownModel; it fires during
INFERENCE rather than on a health check, so a tunnel blip mid-request was enough
to stop a model that was loaded and serving. It consults the transport first
now. A locally spawned backend has no custom transport, so that path is
unchanged byte for byte. Everything else touching a Backend is a consumer rather
than a decorator; there is no fourth.

The probe cache joiner shape is pinned. It was the right design last round with
nothing holding it: the mutation back to a closed-over variable passed all 602
specs in the package. Eight goroutines coalesced on a probe that blocks on a
channel now assert every joiner gets the leader's REASON and not just its
answer, which is the difference between a leader declining to reap and its seven
joiners reaping on the leader's own observation.

The LastDialError scope note claimed an exactness it does not have at
checkIsLoaded, which reads a shared long-lived client after releasing opMutex.
It now says which caller is not exact, why the imprecision is accepted there,
and what making it exact would cost.

The four-outcome table in the docs still said a worker with no live owner is
treated as absent and rescheduled, contradicting the code and the paragraph nine
lines below it. None of those outcomes is absence any more, and the table says
so, names the fifth, and points at the heartbeat as the thing that does decide
presence.

Five mutations, each reddening named specs, including the two the reviewer found
surviving.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
… shape in lint

Re-review round 2. One blocking item, and it was a spec I wrote: eight
goroutines raced at the probe cache and nothing made them coalesce, so a
straggler that missed the flight re-entered the probe and double-closed a
channel. It panicked about one run in three and took the four-suite race block
down. The green verification I reported was not reproducible, which means one
green run was never evidence for a spec that coordinates goroutines. Its comment
claimed the probe blocked until every goroutine was inside flight.Do, and that
gap was exactly the panic: the comment described the design intended rather than
the one written.

It is deterministic now rather than tolerant. singleflight.DoChan registers its
channel on an in-flight call under the group's own mutex and returns without
running its function, so calling it while the leader is provably parked inside
the probe joins that exact flight with no window and no dependence on the
scheduler. The spec asserts the join really happened, that the joiner got the
reason and not only the answer, and that the probe ran once; the entered channel
is sent on rather than closed so a second probe fails an assertion instead of
panicking. Twenty runs green under race against the committed code, five out of
five red on the mutation back to a closed-over variable.

The future-decorator gap is closed in the lint gate, but not the way the review
suggested, and the reason is worth recording. HasMethod rejects inline
signatures outright, its method-reference form needs a package ruleguard's own
typechecker can import and that typechecker cannot import this module, and
Implements tests the value method set while every Unwrap is on a pointer
receiver, so it fired on all three wrappers that already had one.

So the safe shape is structural instead. grpc.WrappedBackend gives the same
pass-through method set plus Unwrap on a value receiver, and a decorator that
embeds it is transparent by construction; forgetting stops being expressible
rather than merely discouraged, which is the move loopbackService already makes
in the worker. FileStagingClient and ConnectionEvictingClient embed it and their
hand-written Unwrap methods are gone. The ruleguard rule then only has to catch
the raw embedding, needs no type filter, and cannot misfire. It was verified to
fire on a throwaway wrapper and stay silent on a correct one, and reports
nothing across core and pkg with the baseline disabled.

InFlightTrackingClient is the one exception and says why in a nolint: it embeds
ControlBackend deliberately so that leaving an inference method unwrapped breaks
the build, and WrappedBackend embeds the full interface, so adopting it would
silently restore pass-through for every inference method and delete that
guarantee.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…y-proof

I reported that enabling gocritic pushed make lint past 600s. That was wrong,
and it was wrong in a way worth naming: those runs happened right after I
changed pkg/grpc's and core/services/nodes' interfaces, so the Go build cache
was cold for essentially the whole repository including every backend, and test
suites were running concurrently on the same machine. I attributed a cold-cache
full-repo typecheck under load to the linter I had just enabled, and raised it
as a cost without ever timing it against a baseline. A number with no control is
not a measurement.

Measured properly, with the golangci cache cleaned before every run and isolated
GOCACHE directories for the cold ones so the shared cache was not wiped: warm,
base 15s then 7s and current 8s then 7s; cold, base 87s and current 78s running
base first, base 136s and current 79s running current first. The spread between
the two cold base runs is larger than any gap between base and current, so
gocritic with only the ruleguard checker costs nothing measurable.

So the rule stays, unscoped. Scoping it to core and pkg was the fallback for a
cost that does not exist, and adding that configuration would buy nothing.

The one override gets the protection it needs instead. InFlightTrackingClient's
nolint is exactly the kind of thing a later reader tidies away, so it now opens
by saying not to, and states what breaks rather than what is intended:
WrappedBackend embeds the full Backend interface, so adopting it there would
promote every inference method as untracked pass-through, the build would stay
GREEN, and in-flight accounting would silently stop covering whatever was added
next. WrappedBackend's own doc carries the counterpart warning so a reader
arriving from either side finds it.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
A worker now opens no listener on a routable interface and states no endpoint at
registration. Backend processes and the file-transfer server bind loopback, and
the frontend reaches both through the tunnel the worker dials. The bind address
is built from loopbackHost, the same constant the tunnel's grpc tag dials, so
"the worker binds where its tunnel dials" is one fact in one place rather than
two literals that can drift.

All three advertisement sites are closed, not one: the registration body,
RegisterNodeRequest, and the per-backend address in the install reply.

That third one was hiding a live bug. stopModelExact refuses a stop whose
ExpectedAddress does not match what the worker recorded for the process. The
worker recorded 127.0.0.1:port; handleBackendInstall reported advertiseHost:port;
the router stored the reported one and sent it straight back. On any worker whose
advertise host was not 127.0.0.1, every acknowledged model stop failed with an
address mismatch. Nothing caught it because the e2e harness set
LOCALAI_ADVERTISE_ADDR=127.0.0.1, which made the rewrite a no-op. Removing the
rewrite makes the two strings the same by construction.

The brief was wrong about two of the four functions it called dead.
effectiveBasePort is the base of the backend port allocator and resolveHTTPAddr
is the file server's bind address; deleting them would have deleted the port
allocator and the file server. Only the two advertise* helpers were dead, and
addr_test.go is rewritten rather than deleted, because the port arithmetic it
pinned still needs pinning.

NodeModel.Address survives with a narrowed meaning and is renamed
WorkerLocalAddress, along with the install reply field that feeds it. The
frontend still has to say WHICH backend process on a worker it means, and the
port in this string is how it says it: it travels as a stream target and the
worker dials its own loopback. The gorm column and the json key stay "address",
so neither a migration nor an API break rides along. Every fall-back to the
node's address is gone. installBackendOnNode now errors when a worker reports
success without naming one, because substituting the now-always-empty node
address would name an empty target, and the worker refuses that as an invalid
stream, which is classified as the worker answering about its backend. That is
the "a present worker reads as something it is not" class this phase forbids.

DistributedModelStore.Range had the same shape and was already wrong: it built
each remote model's client from the node's base gRPC port, never the port a
backend process listens on, so Free and Status went to the wrong place. It uses
the replica's address now.

BackendNode.Address and HTTPAddress are kept but made provably inert: no writer,
no reader that acts on them, and Register force-clears both on re-registration so
an upgraded worker's stale advertisement does not outlive its own upgrade in the
API and the Nodes page. Dropping the columns is a ~90-site edit across the specs,
the e2e suite, the MCP dto and the UI; it is recorded as a follow-up rather than
folded in here.

A persistent tunnel 401 still does not trigger re-registration, and now for a
reason rather than a deferral. Register CLEARS the node's replica rows, so
re-registering on a 401 would delete a live worker's rows on every retry, and
under the name collision that causes the 401 the two workers would take turns
doing it forever: a credential failure causing model reclamation. It also cannot
fix the named cause, since a collision is indistinguishable from a restart. The
401 log now names both causes and says nothing can reach this worker, which is
true only now that it has no listener.

The container healthcheck did not break the way the brief expected, since the
listener still exists on loopback and the probe runs inside the container. It did
have a real #10987 defect that this change makes the common case: it read
LOCALAI_SERVE_ADDR only, while effectiveBasePort reads LOCALAI_ADDR first, so a
worker on a non-default base port was probed on 50050 and reported unhealthy
while working. It follows the same precedence now.

Docs, the compose file and the e2e harness are updated in step: no inbound rule
or published port is needed for a worker, the two advertise variables are gone,
the remaining address variables are read for their port only, the
firewall-the-file-transfer-port warning is narrowed to the LOCALAI_HTTP_ADDR
opt-out, and the upgrade-order note no longer claims the worker still listens.
The Nodes page showed node.address, which is now always blank, so it shows the
node id instead.

Eight mutations, all red on a named spec, including reverting the loopback bind,
re-adding the address to the registration body, restoring both node-address
fall-backs, dropping the force-clear, storing the endpoint's address again, and
un-fixing the healthcheck. One of them caught a defect in a spec I had just
written: it asserted 200 where the endpoint returns 201, which went unnoticed
because core/http/endpoints/localai is not on the task's verify list. It is run
here.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…was refused

Review round 1 on the change that stopped workers listening. One blocking item
and seven notes.

LOCALAI_WORKER_TUNNEL=false was the blocking one, and the ruling was to make it
fatal rather than to correct the comment that still promised it fell back to the
advertised address. There is no fallback left: a worker on this branch
advertises nothing and binds only loopback, so turning the tunnel off leaves it
reachable by nothing while it registers, heartbeats and reports healthy, and the
scheduler keeps placing models on it. That is the worst available failure shape,
so a new Config.validateStartup refuses it before prefetch, registration and
NATS, while the worker is still invisible to the cluster. It absorbs the
pre-existing empty-registration-token check, which had the same shape and no
spec. The flag is kept rather than deleted so an operator who set it is told the
promise is gone instead of having the setting ignored, and the guard around
StartTunnel is removed, because a branch nothing can take reads as a supported
no-tunnel mode that does not exist.

The justification for erroring on an install that names no address was wrong,
and the review is right that this is the dangerous form of overclaiming, because
the conclusion holds and the mechanism does not. It said the resulting empty
target would be refused as an invalid stream and that the refusal would read as
the worker answering about its backend. Nothing in this repo branches on
cluster.ErrNoRoute, and nodes.unroutable treats any recorded dial error as
unroutable, so that refusal reaches every reap guard as ProbeUnknown and deletes
nothing. The site now stands on what holds, that an install naming no port
produced nothing routable and the failure belongs to the install rather than to
a later probe, and records the retracted claim so nobody re-derives it. This
retracts the same paragraph in the body of 1cf847f.

The reviewer deleted the whole tryWarmPath unnamed-replica guard and the suite
stayed green, including the reservation release. It is specced now, and the
asymmetry the review asked about is decided at the site: the row stays, unlike
the sibling !alive branch which removes it. That branch has observed a backend
dead; this one has observed only that the row is unreadable, which says nothing
about whether a process is running, and the row is the last record that one
might be, since the acknowledged stop path refuses a stop whose ExpectedAddress
does not match and an empty one cannot be cleaned up through it either.

The cross-version wire claim rested on two struct tags nobody asserted:
renaming only the json keys survived mutation while the gorm column rename went
red through raw SQL. Both keys are pinned now, marshal and unmarshal, per
struct.

A worker-first upgrade showed the operator a status code and not the reason. The
registration client discarded the body, so "address is required for backend
workers" was read off the socket and thrown away, and the ladder then spent four
minutes on a verdict the frontend reached instantly. Refusals now quote the body
and carry ErrRegistrationRejected, and both the ladder and the credential
manager's Acquire stop on the first one. Acquire matters more than the ladder:
it is the default path and its bound is 100 attempts, not 10. 408 and 429 are
deliberately not refusals, since both are the frontend asking for the same
request again.

Also: the stale "not blocked by firewalls" troubleshooting line, which now names
the real cause and the knobs that move the port range; and the inert address
fields on the MCP Node DTO, which the Assistant was still being handed. The
review named http_address there and I removed address too, because it is inert
by the same argument and leaving one of a pair is arbitrary.

Five mutations, all red. Deleting the warm-path guard reddens four specs and
falsifying only its reservation release reddens one, so the two halves are
pinned separately. Renaming only the json keys reddens both wire suites.
Discarding the refusal body reddens two. Dropping the rejection classification
does not fail the suite, it hangs it, which is the operator-visible symptom, so
it is recorded red under a ginkgo timeout.

The verify list is now derived from the diff rather than from the brief, which
is what let the previous round ship a spec asserting 200 where the endpoint
returns 201: nine ginkgo suites, the e2e vet, route auth coverage, the leaf
check, build, the healthcheck shell suite and lint. The two jsx files have no
harness in this worktree and are recorded as the one unverified surface.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…erence

Everything this phase built was proven by unit and integration specs. This is
the first run of it against the real binaries: a frontend replica per process,
a worker that binds nothing routable, real inference over the result.

Four scenarios, each with the question "what would make this pass if the tunnel
were doing nothing" answered rather than left open.

A worker with no advertised address is reached through its tunnel. The roster
is asserted to report it advertising nothing, so there is no address a frontend
could have dialled instead, and node_connections is asserted to name the
replica that serves the request.

A request landing on the replica that does NOT own the worker is relayed to the
one that does. With N replicas behind round robin that is (N-1)/N of production
traffic, so it gets the FIRST request for its model: the backend install, the
file staging on the http tag, and the gRPC load and predict all cross the
relay. Which replica owns the tunnel is read from the ownership table through
the production Owner query and mapped to a frontend index through the address
the harness pins per replica; the non-owner is derived from that reading and
asserted to be a non-owner immediately before the request, rather than assumed
from the harness default. Sending the same request to the owner reddens it.

Killing the owning replica re-homes the worker onto the survivor. The worker
dials a balancer rather than a replica, because LOCALAI_REGISTER_TO is resolved
once at boot and is the tunnel endpoint as well as the registration one: aimed
at a single replica, a worker has nowhere to reconnect to when that replica
dies, and the re-home cannot happen at all. Removing the kill reddens it.

And the negative control for the whole suite, which is why the other three mean
anything. Frontend and worker share a host here, so every backend port the
frontend names in a stream target is one it could have dialled directly; if it
did, the first three would pass with the tunnel inert. LOCALAI_WORKER_TUNNEL is
no longer usable for this, because it is a fatal startup error and a worker that
never started says nothing about a worker reachable some other way. The balancer
answers the tunnel connect path itself instead, leaving a worker that registers,
heartbeats, reports healthy and holds no tunnel. It is asserted to have dialled
and been refused, asserted to be held by nobody, and then asserted unreachable
with the refusal naming the missing route. Then the block is lifted, nothing
else changes, and the same request succeeds: that is what attributes the refusal
to the tunnel rather than to any of the ordinary reasons an e2e inference fails.

The fifth spec measures the head-of-line blocking this phase deferred three
times. 128 MiB crosses the session while a warm model is probed back to back,
direct and relayed. Median latency is unchanged, the worst probe is about 3x the
baseline median and about a seventeenth of the transfer window, and the transfer
runs at 415-490 MB/s direct and 222-268 MB/s relayed. A session that
head-of-line blocked would park a probe for the length of the window. Leave the
yamux windows untuned; and note this is loopback, so it says the multiplexing
does not serialise and says nothing about a link with a bandwidth-delay product.

The load spec is measured against a control that the first version did not have.
It passed with the bulk artifact cut to 4 KiB, because the window it read probes
against was mostly cold-load overhead: it would have reported a clean bill on a
session carrying no large message. The same cold load now runs twice, once
empty and once bulk, and the difference between the windows is asserted to be
real before any latency is read from it.

Two defects on the base commit came out of this.

cluster_peerlink_test.go has been red since the relay landed, deterministically,
in isolation and in the suite. It asserted that an accepted peer stream is
refused at once, on the premise that phase 1 installs no relay. The relay
correctly waits fifteen seconds for a frame naming the worker, and the spec's
budget was five. It now writes a relay request for a node no replica holds and
asserts the refusal is ErrNotOwner and specifically not ErrNoConnection, which
is a stronger spec than the one it replaces and the only thing in the e2e suite
that exercises the relay's refusal path.

The harness handed a worker's own HTTP port to a backend process. It took two
ports from freeport and used one as the gRPC base and the other for the file
transfer server; freeport returns adjacent ports often, and the backend
allocator hands out base, base+1, base+2, so the second backend started on a
worker was regularly given the HTTP server's port and died with EADDRINUSE. No
spec had started two backends on one worker before, so it had never fired; the
load spec starts five and it failed about one run in three. Each worker now
reserves a contiguous bind-probed block laid out the way production lays it out,
below the kernel's ephemeral range, with LOCALAI_GRPC_MAX_PORT bounding the
allocator to it. The underlying production defect is not fixed here and is
recorded in the report: allocatePort never checks that a port is free, and its
default range overlaps the ephemeral range on every Linux box.

Constraint 6, whether distributed mode should now refuse to start without an
advertised address, is DEFERRED, and the comment and the docs that described the
cost were understating it. A replica with no advertised address writes no
instances row, and Owner joins a connection against a live instance, so a worker
whose tunnel lands there is unroutable from every OTHER replica while being
registered and healthy. Refusing to start would still be wrong, because the
deployments it would break are single-host ones with no peers to be unreachable
by, and telling those apart at startup is a design with its own specs. Both
places now say what actually happens.

Suite wall clock 592s for 15 specs, up from 502s for 10 of which 2 were red. The
CI budget of 20 minutes does not move.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Review round 1 on the end-to-end proof. Zero blocking items, eleven
non-blocking, and three of them turned out to be production defects rather
than notes on the report.

The one that matters is a misclassification the phase is built to prevent. A
dial carries the caller's deadline down to the socket, so when the budget runs
out the socket's timer fires and the error travels back up through the
WebSocket handshake and the multiplexer. The context's cancellation is a
separate timer whose func the scheduler has to run before ctx.Err() stops
returning nil, and nothing orders the two. Under contention the socket's error
is back in PeerPool.Open first, ctx.Err() reads nil, and a peer that is
listening and healthy is reported as ErrPeerUnreachable to a caller that simply
ran out of time. An unreachable peer is a fact a caller may act on and an
expired deadline is not, and core/services/nodes routes around a replica it is
told is unreachable.

callerRanOut answers that question in one place: ctx.Err() when it is set, and
otherwise the wall clock against the caller's own deadline. That is sound
because it is the same instant the socket compared itself against, so if the
socket's timer fired this comparison is past it too. The ambiguous instant
resolves towards the caller, which is the direction that never blames a peer.

The spec that caught it, peerlink_test.go's "blames the caller's deadline",
was red in three of seven -race runs and had been since Task 5, which is often
enough to read as noise and is why single-run verification never saw it. Rather
than leave the proof to a coin flip, a second spec makes the window
deterministic: Open is handed a context whose deadline has passed and whose
cancellation has not been delivered, against an address nothing is listening
on, so the dial fails for real. It reddens without the fix.

The peer link's yamux windows were applied to one end only. A receive window is
advertised by the side that RECEIVES, so configuring the dialler alone tunes
exactly one direction, and the direction left on the 256 KiB default is the one
that carries a relayed model artifact INTO the replica that owns the worker's
tunnel. That is the largest thing the link ever moves and it is the direction
the load measurement exercises: the review read it as flowing toward the
dialler and it does not. PeerLinkConfig is now exported and used on both ends.
Measured, same box, 128 MiB staged through the relay against the same transfer
without one: the relayed path cost 1.6x to 2.0x the direct path's transfer
window before, and 1.06x to 1.25x after.

The SSRF reachability spec could be fooled into reporting an SSRF that did not
happen. It bound the victim on 127.0.0.2 at an ephemeral port and required
127.0.0.1 at the same port to refuse, so any other spec in the run holding that
number made the dial succeed; red one run in seven, green five of five in
isolation. It now picks from below the kernel's ephemeral range, the same fix
the harness got for the adjacent-port collision.

The rest are the specs and the report saying what they mean.

Scenario 1's advertisement assertion could not tell "the worker advertises
nothing" from "the JSON key moved", which matters because removing the
advertisement is the change it covers. It was green against a renamed key. The
roster now keeps the raw key set beside the decoded fields and the spec
requires both keys present before reading them as empty.

Scenario 4's refusal-body check was a four-way disjunction admitting bare
"tunnel", "not connected" and "unroutable". Those alternatives were inert and
each would be satisfied by refusals that say nothing about routing, in the one
assertion the whole negative control rests on. It is "no route" alone.

The head-of-line gate bounded the worst probe by the whole transfer window,
which admits about eightfold degradation and loosens as the box slows. It is
now half the window, plus a scale-free ratio against the worst probe under the
SAME cold load with nothing to transfer, which is the control that isolates the
transfer from the load. Not tighter than that, and the reason is measured
rather than cautious: under a concurrent -race suite the worst relayed probe
reached a fifth of its window, so a quarter-window gate would have had 1.2x of
margin, and a spec that fails one run in three is worse than no spec.

The report entry printed p90 and p99 off samples of twenty, where both land on
the same element and p99 often lands on the max, so one number appeared three
times under three names. A quantile is now printed only when the sample can
separate it.

Two claims in the report were wrong and are withdrawn rather than softened.
Scenario 2's race is closed by the trailing re-read of the owner, not by the
pre-assertion the report credited: a move to the non-owner mid-request would
serve directly and still return 200, and only the trailing read reddens on it.
And "the median request is unchanged" holds on this box and not on the
reviewer's, where the relayed median rises up to 82% and p99 up to 3.5x. What
survives on both is structural: the worst probe is a small fraction of the
window in which bytes are moving, so the session interleaves rather than
serialising. Sharing a session with a bulk transfer costs latency; it does not
cost service.

The disk footprint note undercounted, and the reviewer lost a run to a full
disk on this box, so it is worth having right: two bulk models seeded into two
frontends and staged to the worker is about 768 MiB, not 512 MiB.

Left alone deliberately: the worker's backend port allocator still hands out
ports without checking they are free, and its default range still overlaps the
kernel's ephemeral range. It is confirmed, it is out of scope here, and it is
being tracked as a named follow-up rather than fixed under an e2e task.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…ckend

A worker that refuses a stream has answered, and cluster.Dial keeps the three
tunnelproto sentinels out of the ErrNoRoute umbrella precisely so a consumer
can act on that. No consumer did. Since workers stopped listening, a backend
process that crashed on a healthy worker is no longer a dead listener's
codes.Unavailable: the worker refuses the stream with
ErrStreamTargetUnavailable, gRPC flattens it into Unavailable anyway, and
nodes.unroutable reported the whole thing as "this frontend has no route".
Every reap path then answered ProbeUnknown and left the row, so the replica
slot never freed and at the default MaxReplicasPerModel=1 the only cleanup
left was LRU eviction of models that were working.

isWorkerAnswer is exported as cluster.IsWorkerAnswer, so the errors the dialer
keeps out of the umbrella are by construction the errors the consumers treat
as the worker answering. nodes.unroutable and pkg/model's transportFailure
both use it; ConnectionEvictingClient, the site reached during inference, goes
through transportFailure rather than asking the transport directly. A reply
code this frontend does not recognise is still not an answer, so a newer
worker's vocabulary costs a retry and not a replica.

The reap guards keep the allow-list rather than requiring ErrNoRoute: an
unrecognised dial error must mean "no route", never "the backend is gone".

Also in this final pass over the branch:

- Docs: recommend upgrading FRONTENDS first, with the symptom of each order.
  Workers-first fails now that a 4xx registration is a verdict rather than an
  outage, so an old frontend's "address is required for backend workers" makes
  each restarted worker exit and drains the fleet a node per restart.
- Docs: LOCALAI_WORKER_TUNNEL=false is a fatal startup error, not a degraded
  mode, in both places that described it; and a frontend rollback needs every
  worker restarted, because re-registration force-clears the address columns.
- A replica with no advertised address now says so every five minutes and
  names the workers only it can reach, instead of one startup warning for a
  cost paid for the life of the process.
- callerRanOut's rule now holds at all three siblings, so an expired caller
  deadline stops reading as a broken tunnel; probeHealth's withdrawn reason
  for using the raw client is corrected; the dead DoOrCached is deleted and
  its coverage kept on DoOrCachedResult; sweepLeakedInFlight enumerates the
  outcomes that reach it.
- The peer route's self-declared id is recorded as a phase-3 deferral, in the
  handler, in the isolation claim it narrows, and in the operator docs.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Making a worker's refusal reaping evidence created a defect one layer
along, at the producer. The worker refused a ReadStreamRequest failure with
ErrStreamRequestInvalid and its own comment said "Includes the deadline
above expiring", which was harmless while every refusal reached the
frontend as "no route" and became a reap the moment one of them did not. So
a request frame that had merely not ARRIVED yet was reported as a
non-transient verdict about a backend.

It is reachable on the relay path, which carries most production traffic:
the worker's header timer starts when the OWNING replica opens the stream,
while the frame is written by the DIALLING replica only after the relay's
acceptance travels back to it, so a whole peer-link round trip runs inside
that window, on a link this design deliberately loads with multi-gigabyte
artifacts beside token streams. For a long-deadline caller the endpoint is
ConnectionEvictingClient, which stops the model across the fleet. It also
falsified the "neither clears on its own" argument that licensed the reap.

There is now a fourth refusal, ErrStreamNotServed, for what a worker could
not serve for a reason of its OWN. It is deliberately outside
IsWorkerAnswer, so it reaches a consumer under the no-route umbrella and
reaps nothing, which is the same treatment an unrecognised code already
gets. Four producers move onto it: a request frame that timed out (a
malformed one stays a verdict, because that is a frontend bug no retry
fixes), both SetReadDeadline failures, which are facts about the stream and
not about a target nothing has dialled yet, and WriteStreamRefusal's
default for a reason nobody classified.

classifyServiceFailure keeps ErrStreamTargetUnavailable as its default on
purpose: inverting it would make errno enumeration the single point of
failure for the reap, and a miss there is a row nothing can ever delete.
What it gains is a deny-list of two causes that are provably this worker's
own clock or its own context.

Also:

- The read-site caller-deadline guard in the handshake was unpinned: the
  existing seam spends the budget before the handshake starts, so only the
  write could ever fail. A spec whose deadline falls between the request and
  the reply pins it, and each guard now reddens on its own.
- The documented worker-first failure line omitted the JSON error envelope
  the old frontend returns, so an operator grepping it found nothing.
- The peer-link disclosure names the aimable per-session receive window in
  all four places, and LastDialErrorOf records why a third consumer must go
  through IsWorkerAnswer rather than roll its own list.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
The worker re-classified a failure a local service had already classified.
classifyServiceFailure preserved exactly one of the four refusal codes,
which was faithful to its own comment for as long as there was one worth
keeping; once ErrStreamNotServed existed, a service returning the code
whose whole job is to say "I learned nothing" had it promoted to
ErrStreamTargetUnavailable, which every reap guard acts on.
ErrStreamTagUnknown was promoted too, and cost nothing only because both
sides of that one reap. No in-tree service produces either, which is the
same "unreachable, therefore safe" argument that let the request-frame
merge survive a whole phase, and LocalService is exported.

The cause was a fifth site enumerating the vocabulary by hand, so the fix
is one table. streamRefusals pairs each sentinel with its wire code and
with whether a frontend may act on it as evidence about a backend, and the
writer, the reader, IsWorkerAnswer and the new IsStreamRefusal all read it.
A fifth code is now taught to every one of them at once.

The codes are also pinned against literals written out in a spec, the way
this branch already pinned the NATS vocabulary. The round-trip table
cannot see a rename, because a rename moves the writer and the reader
together; an unrecognised code is deliberately not the worker's answer, so
renaming "unavailable" would turn every crashed backend on a tunnelled
worker into a row nothing can ever reap, silently and with the suite green.

Three comments the previous fix falsified, corrected:

- tunnelHeaderTimeout still said the window bounds only framing the
  frontend writes immediately after opening the stream. That is true on the
  direct path and false on the relay path, and it was the argument for
  treating an expiry as the frontend's fault.
- classifyServiceFailure's deny-list is three causes, not two: on a dial
  error net.Error.Timeout also covers ETIMEDOUT and EAGAIN. Both are kept
  deliberately, because reaping a wedged or resource-starved backend is the
  eviction this phase exists to prevent, and ECONNREFUSED still reaps.
  isReadTimeout is renamed reportsTimeout, which is what it asks.
- The operator table named three refusals and said a refusal is acted on.
  It now lists four, with when each is sent and whether the row is reaped.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
return fmt.Errorf("tunnel frame is %d bytes, over the %d-byte limit", len(payload), maxTunnelFrame)
}
buf := make([]byte, 2+len(payload))
binary.BigEndian.PutUint16(buf[:2], uint16(len(payload)))
d = scaled
}
}
return d/2 + time.Duration(rand.Int64N(int64(d/2)+1))
}
for file, content := range c.opts.Models {
path := filepath.Join(dir, "models", file)
if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
// observed".
func reserveWorkerPorts() (int, error) {
for attempt := 0; attempt < workerPortAttempts; attempt++ {
base := workerPortFloor + rand.IntN(workerPortCeiling-workerPortFloor)
@mudler
mudler merged commit 6b712e7 into test/distributed-e2e-ci Sep 2, 2026
236 of 242 checks passed
@mudler
mudler deleted the feat/worker-tunnel-phase1-2 branch September 2, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants